-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add Visual JSON Schema Editor #905
feat: add Visual JSON Schema Editor #905
Conversation
|
✅ Deploy Preview for modest-rosalind-098b67 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for asyncapi-studio-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
83ef1ca
to
f8d508a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dude that was fast 👏
@asyncapi/bounty_team |
@princerajpoot20 |
4f6d5b4
to
b2a619c
Compare
Currently working on:I am working on resolving type error that is causing the build to fail, though it runs on my local machine. |
Is something blocking this PR from merging, though the functionality is not finished, but agreed to be merged 'as is?' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/rtm |
@princerajpoot20 could you check @fmvilas suggestion: #905 (comment) if it's not relevant anymore let him know so he can approve as well We can merge "as is" and make a new iteration to adjust it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. I'm trying to use the component in the design system (in Storybook) and it doesn't work when I try to remove fields or mark them as required. Is it a thing of how we're integrating it in Storybook or is it a thing of the component? Would love to see it working completely as if it was in the real app.
Also, the design still needs some work. We should make it match the proposed design IMHO.
That said, I'm not a code owner of Studio. If code owners think we should merge this, I'm happy to dismiss my review. That's totally fine, we can address these issues later down the track. Just wanted to highlight the things I'm seeing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just got a DM from @princerajpoot20 explaining the situation:
I’ve discussed this with Samir, and we’ve decided to complete it in two iterations.
We’ve adjusted the complexity of the bounty from advanced to two medium bounties.
I have completed the first iteration, and the second iteration is scheduled for Q2 of 2024 (Link for 2nd iteration).
This iteration will include all remaining tasks, such as deletion, toggling between required/not required, and UI changes. Hence we are merging the current progress now and will complete the remainder in the second iteration.
Approving! 👍
Quality Gate passedIssues Measures |
/rtm |
Adding Visual JSON Schema Editor
Resolves: #755